Skip to content

Fix SearchSpaceAnalysis crash on non-numeric ordered choice parameters#4991

Closed
eonofrey wants to merge 1 commit into
facebook:mainfrom
eonofrey:export-D94979372
Closed

Fix SearchSpaceAnalysis crash on non-numeric ordered choice parameters#4991
eonofrey wants to merge 1 commit into
facebook:mainfrom
eonofrey:export-D94979372

Conversation

@eonofrey
Copy link
Copy Markdown
Contributor

@eonofrey eonofrey commented Mar 6, 2026

Summary:
search_space_boundary_proportions assumed all ordered ChoiceParameter values
are numeric (int | float), but some expeirments have ordered string values (somehow?). When this happens, the SearchSpaceBoundary check breaks (example).

This diff adds a check that all values are numeric before entering that branch,
so non-numeric ordered choice parameters are skipped like unordered ones.

Reviewed By: ItsMrLin

Differential Revision: D94979372

Summary:
`search_space_boundary_proportions` assumed all ordered `ChoiceParameter` values
are numeric (`int | float`), but some expeirments have ordered string values (somehow?). When this happens, the SearchSpaceBoundary check breaks ([example](https://www.internalfb.com/ax/experiment/p_wyt_directional_v7_ax/experiment)). 

This diff adds a check that all values are numeric before entering that branch,
so non-numeric ordered choice parameters are skipped like unordered ones.

Reviewed By: ItsMrLin

Differential Revision: D94979372
@meta-codesync
Copy link
Copy Markdown

meta-codesync Bot commented Mar 6, 2026

@eonofrey has exported this pull request. If you are a Meta employee, you can view the originating Diff in D94979372.

@meta-cla meta-cla Bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Mar 6, 2026
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.83%. Comparing base (9f50a93) to head (3ec1c87).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4991      +/-   ##
==========================================
- Coverage   96.83%   96.83%   -0.01%     
==========================================
  Files         599      599              
  Lines       64584    64590       +6     
==========================================
+ Hits        62540    62543       +3     
- Misses       2044     2047       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@meta-codesync
Copy link
Copy Markdown

meta-codesync Bot commented Mar 6, 2026

This pull request has been merged in eb264f1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Do not delete this pull request or issue due to inactivity. fb-exported Merged meta-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants